feat: require with_environment_subdomain or an explicit with_legacy_domain opt-out - #193
Conversation
🟡 Risk Classification: MINORApproval route: AI Review + Human Approval Classification reasons
Operational gates
Files analysed: 12 wall-e 2026.06.19-02 · policy |
🔬 Debug — why this classification?Each reason code emitted by the classifier, its source clause in the AI in SDLC Control Framework, and what it means.
Kinds:
See issue #3 for the proposal to formalise this map as Appendix A of the standards doc. wall-e 2026.06.19-02 · debug |
🔵 Advisory review: Sound, but needs your judgementThis PR needs a human approval. The code itself reads as correct; whether it should land depends on context I don't have. This PR enforces mandatory merchant-specific subdomains for Default/OAuth platforms with a legacy-domain opt-out escape hatch, and the implementation looks technically correct — but the test suite itself universally opts out of the very feature being made mandatory, which is a consequential deployment/readiness question. For you to decide
This is not an approval. wall-e cannot auto-approve this PR — it is an opinion to help whoever does. Advisory review · us.anthropic.claude-sonnet-4-6 · wall-e 2026.06.19-02 |
🔴 Risk Classification: MAJORApproval route: AI Review + Human Approval Required Classification reasons
Operational gates
Files analysed: 15 wall-e 2026.06.19-02 · policy |
🔬 Debug — why this classification?Each reason code emitted by the classifier, its source clause in the AI in SDLC Control Framework, and what it means.
Kinds:
See issue #3 for the proposal to formalise this map as Appendix A of the standards doc. wall-e 2026.06.19-02 · debug |
🔴 Risk Classification: MAJORApproval route: AI Review + Human Approval Required Classification reasons
Operational gates
Files analysed: 14 wall-e 2026.06.19-02 · policy |
🔬 Debug — why this classification?Each reason code emitted by the classifier, its source clause in the AI in SDLC Control Framework, and what it means.
Kinds:
See issue #3 for the proposal to formalise this map as Appendix A of the standards doc. wall-e 2026.06.19-02 · debug |
🟡 Risk Classification: MINORApproval route: AI Review + Human Approval Classification reasons
Operational gates
Files analysed: 9 wall-e 2026.06.19-02 · policy |
🔬 Debug — why this classification?Each reason code emitted by the classifier, its source clause in the AI in SDLC Control Framework, and what it means.
Kinds:
See issue #3 for the proposal to formalise this map as Appendix A of the standards doc. wall-e 2026.06.19-02 · debug |
…omain opt-out The merchant-specific subdomain is how merchants should reach the API, but it was optional and an unset value silently fell back to api.checkout.com, so a forgotten subdomain looked exactly like a deliberate opt-out and the SDK could not warn about either. Callers must now choose: call with_environment_subdomain, or with_legacy_domain, which prints a deprecation warning from its first release. Both, or neither, raises CheckoutArgumentException. An invalid subdomain now raises instead of being quietly ignored, which is a second breaking change: callers passing a malformed value are currently served by the shared host and never find out. with_environment_subdomain no longer needs with_environment to be set first, since the EnvironmentSubdomain is now built when the configuration is assembled. The Previous (ABC) platform predates merchant-specific subdomains and stays exempt via requires_environment_subdomain?. Specs route clients through Helpers::DomainConfiguration, which uses the shared hosts: the sandbox OAuth clients are not provisioned for the subdomain, so applying it makes every client_credentials request return invalid_client. Mirrors checkout-sdk-net#590. Refs INT-1688.
The suite could only run against the shared hosts, so the subdomain path this PR makes mandatory had no integration coverage. Reviewers flagged that on every SDK, and it is the right thing to flag. The domain helper now has two modes. Default is unchanged, the shared hosts, because the sandbox OAuth clients are not provisioned for the subdomain and the token request returns invalid_client. Set CHECKOUT_TEST_USE_SUBDOMAIN=true and the suite runs against CHECKOUT_MERCHANT_SUBDOMAIN instead, so once sandbox is provisioned like production it is a one-line change in the workflows, already wired and documented, rather than a rewrite of every fixture. The switch is deliberately separate from CHECKOUT_MERCHANT_SUBDOMAIN, which CI already exports: provisioning should drive the behaviour, not the presence of a secret.
Versions are bumped on master during the release, not in a feature branch, per the release workflow. This branch should carry only the change itself; the major bump is classified and applied when the release is cut.
Two problems with the previous approach. It needed a new variable in 21 workflow files, which is not viable without access to create secrets. And it wrapped the builder chain in a configureDomain helper that is not part of the public API, so the tests stopped looking like the code a merchant would actually write. Every fixture now calls the real opt-out inline, in the chain, with a comment saying why: the sandbox OAuth clients are not provisioned for the merchant-specific subdomain, so the token request comes back invalid_client. When sandbox is provisioned, those calls become the subdomain setter. The unit tests covering all four combinations are untouched: they already used the public API directly.
c072be4 to
7d77416
Compare
🟡 Risk Classification: MINORApproval route: AI Review + Human Approval Classification reasons
Operational gates
Files analysed: 10 wall-e 2026.06.19-02 · policy |
🔬 Debug — why this classification?Each reason code emitted by the classifier, its source clause in the AI in SDLC Control Framework, and what it means.
Kinds:
See issue #3 for the proposal to formalise this map as Appendix A of the standards doc. wall-e 2026.06.19-02 · debug |
🟡 Risk Classification: MINORApproval route: AI Review + Human Approval Classification reasons
Operational gates
Files analysed: 9 wall-e 2026.06.19-02 · policy |
🔬 Debug — why this classification?Each reason code emitted by the classifier, its source clause in the AI in SDLC Control Framework, and what it means.
Kinds:
See issue #3 for the proposal to formalise this map as Appendix A of the standards doc. wall-e 2026.06.19-02 · debug |
…th client The dedicated sandbox clients are not provisioned for the merchant subdomain; the default client now carries every scope the suites need.
🟡 Risk Classification: MINORApproval route: AI Review + Human Approval Classification reasons
Operational gates
Files analysed: 9 wall-e 2026.06.19-02 · policy |
🔬 Debug — why this classification?Each reason code emitted by the classifier, its source clause in the AI in SDLC Control Framework, and what it means.
Kinds:
See issue #3 for the proposal to formalise this map as Appendix A of the standards doc. wall-e 2026.06.19-02 · debug |
…ault OAuth client" This reverts commit 3b42976.
🟡 Risk Classification: MINORApproval route: AI Review + Human Approval Classification reasons
Operational gates
Files analysed: 9 wall-e 2026.06.19-02 · policy |
🔬 Debug — why this classification?Each reason code emitted by the classifier, its source clause in the AI in SDLC Control Framework, and what it means.
Kinds:
See issue #3 for the proposal to formalise this map as Appendix A of the standards doc. wall-e 2026.06.19-02 · debug |
- Anchor the subdomain pattern with \A...\z so subdomains containing trailing or embedded newlines are rejected, and cover both cases with unit specs - Validate the environment settings before the key validation, matching the other SDKs - Build the environment-subdomain object once per build instead of recomputing it on every access - Initialise the legacy-domain flag in the constructor so it is never read before assignment - Reword the subdomain guidance to 'typically your client ID excluding the cli_ prefix' in the error messages - Use the CHECKOUT_MERCHANT_SUBDOMAIN env var for the static-keys integration fixture; keep the legacy-domain opt-out only for OAuth clients, citing the sandbox OAuth provisioning gap - Document that Private Link merchants use their pl- prefixed subdomain
🟡 Risk Classification: MINORApproval route: AI Review + Human Approval Classification reasons
Operational gates
Files analysed: 11 wall-e 2026.06.19-02 · policy |
🔬 Debug — why this classification?Each reason code emitted by the classifier, its source clause in the AI in SDLC Control Framework, and what it means.
Kinds:
See issue #3 for the proposal to formalise this map as Appendix A of the standards doc. wall-e 2026.06.19-02 · debug |
🟡 Risk Classification: MINORApproval route: AI Review + Human Approval Classification reasons
Operational gates
Files analysed: 11 wall-e 2026.06.19-02 · policy |
🔬 Debug — why this classification?Each reason code emitted by the classifier, its source clause in the AI in SDLC Control Framework, and what it means.
Kinds:
See issue #3 for the proposal to formalise this map as Appendix A of the standards doc. wall-e 2026.06.19-02 · debug |
|
2 participants
Summary
Makes the merchant-specific subdomain (MSSD) mandatory. Callers must now call
with_environment_subdomain(...), or explicitly opt out with the newwith_legacy_domain, which prints a deprecation warning from its first release. Setting both, or neither, raisesCheckoutArgumentExceptionat build time. MSSD is no longer beta and non-MSSD usage will be deprecated, so the previous silent fallback toapi.checkout.comhad to go.Changes
lib/checkout_sdk/abstract_checkout_sdk_builder.rb— the subdomain is held as a string and theEnvironmentSubdomainis built when the configuration is assembled, sowith_environment_subdomainno longer has to be called afterwith_environment; newwith_legacy_domain; newvalidate_environment_settingsandrequires_environment_subdomain?lib/checkout_sdk/environment_subdomain.rb—create_url_with_subdomainraises on an invalid subdomain instead of returning the URL unchangedlib/checkout_sdk/previous/checkout_previous_static_keys_sdk_builder.rb— Previous/ABC exemptedspec/checkout_sdk_spec.rb— covers all four combinations, an invalid subdomain, and the Previous exemptionspec/checkout_sdk/configuration/configuration_spec.rb— the bad-subdomain cases now assert the raise instead of the silent fallbackspec/support/domain_configuration.rb(new) +sandbox_test_fixture.rb, the accounts spec and the issuing helper — every client the suite builds now chooses a domainFixed along the way
with_environment_subdomainused to build the URLs from whatever environment was set at call time, so calling it beforewith_environmentsilently produced the wrong host.Verification
576 examples, 0 failures, 149 pending.
rubocopreports no offences in thelib/files touched here; the only new offence overall isMetrics/BlockLengthin a spec file that already exceeded it.API Reference
Breaking changes
Yes, two. This needs a major release, classified and versioned when the release is cut.
api.checkout.com/access.checkout.comnow fails at client construction. Migration: set the subdomain, or use the legacy-domain opt-out as a temporary measure. The Previous (ABC) platform is unaffected.README
Updated in this PR: a "Subdomain value" section above the Default example, the subdomain added to the configuration samples, and a "Legacy domain (emergency use only)" section at the bottom.
Notes
The suite routes every client it builds through a single helper that uses the shared hosts. Applying the merchant-specific subdomain there looked better, since it is the path merchants are being moved to, but the sandbox OAuth clients are not provisioned for it: .NET CI failed 224 integration tests with
invalid_clientwhen the token request went to{subdomain}.access.sandbox.checkout.com. Binding those OAuth clients to the subdomain is a platform task and should land before merchants are told the subdomain is mandatory.Reference implementation: checkout-sdk-net#590. Tracked as INT-1688.
No version bump here: that happens on master when the release is cut, per the release workflow.
Review follow-ups (2026-08-31)
Breaking changes, complete list:
Behaviour note: host resolution is now deferred to build time, which also fixes a latent order-dependence bug where setting the subdomain before the environment produced the wrong host.
Deprecation signal: runtime warn to stderr.
Option A applied (2026-08-31): an explicit OAuth authorization URI and the environment subdomain are now mutually exclusive at build time; the README OAuth example no longer sets an authorization URI, subdomain-only is the documented path.